-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: handle empty responses from Chutes AI API #7323
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add content tracking in ChutesHandler.createMessage() for both DeepSeek and non-DeepSeek models - Throw descriptive error when API returns no content - Add test coverage for empty response scenarios - Fixes #7322
| // If no content was received, throw an error | ||
| if (!hasContent) { | ||
| throw new Error( | ||
| `${this.providerName} API did not return any content. This may indicate an issue with the API, model configuration, or request parameters.`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate error message text appears in both branches. Consider extracting the error string into a shared constant to reduce duplication and ease future maintenance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewing my own code is like debugging in a mirror - everything looks backwards but the bugs are still mine.
| } | ||
|
|
||
| // If no content was received, throw an error | ||
| if (!hasContent) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I notice both branches have identical error handling logic. Could we extract this into a shared helper method to follow DRY principles? Something like:
private throwIfNoContent(hasContent: boolean): void {
if (!hasContent) {
throw new Error(
`${this.providerName} API did not return any content. This may indicate an issue with the API, model configuration, or request parameters.`,
)
}
}| } else { | ||
| yield* super.createMessage(systemPrompt, messages) | ||
| // For non-DeepSeek models, we reimplement the stream handling instead of calling | ||
| // super.createMessage() to ensure consistent error handling for empty responses |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a specific reason we're reimplementing the stream handling instead of wrapping the parent's implementation? We could potentially do something like:
const parentStream = super.createMessage(systemPrompt, messages);
let hasContent = false;
for await (const chunk of parentStream) {
if (chunk.type === 'text') hasContent = true;
yield chunk;
}
if (!hasContent) throw new Error(...);This would reduce code duplication and maintenance burden.
| // If no content was received, throw an error | ||
| if (!hasContent) { | ||
| throw new Error( | ||
| `${this.providerName} API did not return any content. This may indicate an issue with the API, model configuration, or request parameters.`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we make the error message more actionable? For example: "Check your API key validity, model availability, or request parameters (e.g., max_tokens, temperature)."
| [Symbol.asyncIterator]: async function* () { | ||
| // Empty stream for this test | ||
| // Yield minimal content to avoid triggering the empty response error | ||
| yield { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be cleaner to keep the original test unchanged and create a separate test specifically for the empty stream scenario? This modification makes the test less clear about its original intent.
| async next() { | ||
| if (!called) { | ||
| called = true | ||
| // Return minimal content to avoid triggering the empty response error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same concern here - this test modification obscures the original test intent. Consider keeping the original test and adding a new one for the empty response case.
|
Issue needs info |
Description
This PR attempts to address Issue #7322 where the Chutes AI API returns an unexpected error: "The language model did not provide any assistant messages."
Problem
When using Chutes AI (either with the built-in provider or via OpenAI compatible API), the API sometimes returns responses without any content in the stream chunks, causing the application to throw an error about missing assistant messages.
Solution
hasContentflag) in theChutesHandler.createMessage()methodChanges
src/api/providers/chutes.tsto track and validate content presencesrc/api/providers/__tests__/chutes.spec.tsTesting
Fixes #7322
Feedback and guidance are welcome!
Important
Adds content tracking and validation in
ChutesHandlerto handle empty responses from Chutes AI API, with tests for both DeepSeek R1 and standard models.hasContentflag inChutesHandler.createMessage()to track content presence.chutes.spec.tsfor empty response scenarios.chutes.tsto implement content tracking and validation.chutes.spec.tsto include new test cases for empty responses.This description was created by
for 62b49a1. You can customize this summary. It will automatically update as commits are pushed.